Skip to content

Added configurable maxFramePayloadLength to ReactorNettyWebSocketClient #22367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Added configurable maxFramePayloadLength to ReactorNettyWebSocketClient #22367

wants to merge 4 commits into from

Conversation

envious
Copy link
Contributor

@envious envious commented Feb 6, 2019 •
edited
Loading

Currently the ReactorNettyWebSocketClient (RNWSC) doesn't allow for a configurable maxFramePayloadLength. The HttpClient used by the RNWSC allows for a configurable maxFramePayloadLength however, the method .websocket() used by the RNWSC ignores whatever is set by the HttpClient and uses a default value of 65536. As shown below:

ReactorNettyWebSocketClient invokes .websocket()

.websocket(StringUtils.collectionToCommaDelimitedString(handler.getSubProtocols()))

The method invoked on the HttpClient ignores the value you may have set in your HttpClient

public final WebsocketSender websocket(String subprotocols) {
	return websocket(subprotocols, 65536);
}

I've solved the issue by using the overloaded .websocket() method on the HttpClient which takes a maxFramePayloadLength.

.websocket(StringUtils.collectionToCommaDelimitedString(handler.getSubProtocols()), 
getMaxFramePayloadLength())

The maxFramePayloadLength is now configurable in the RNWSC.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 6, 2019
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 8, 2019
@rstoyanchev rstoyanchev added this to the 5.2 M1 milestone Feb 8, 2019
rstoyanchev added a commit that referenced this pull request Mar 12, 2019
@rstoyanchev
Merge pull request #22367 from envious/adding-maxframebuffersize
baa7e2e
@philsttr philsttr mentioned this pull request Mar 18, 2019
Closed
@philsttr
Copy link

I noticed that this PR did not change ReactorNettyWebSocketClient to call the constructor for ReactorNettyWebSocketSession that takes the maxFramePayloadLength

It seems odd that the ReactorNettyWebSocketSession.maxFramePayloadLength is not set on the client side, but is set on the server-side. i.e. ReactorNettyWebSocketSession.maxFramePayloadLength will keep the default value (64k) on the client side regardless of how ReactorNettyWebSocketClient.maxFramePayloadLength is configured.

Is this intended?

Will something bad happen if ReactorNettyWebSocketClient.maxFramePayloadLength has a different value than ReactorNettyWebSocketSession.maxFramePayloadLength ?

@spencergibb spencergibb mentioned this pull request Mar 19, 2019
Closed
rstoyanchev added a commit that referenced this pull request Mar 19, 2019
@rstoyanchev
ReactorNettyWebSocketSession uses correct frame size
a2fcf0a 
See gh-22367
@rstoyanchev
Copy link
Contributor

Good catch @philsttr, thanks.

@eliquinox
Copy link

Is it possible to use this feature now or do we have to wait for the milestone?

@benitocm

This comment has been minimized.

Sign in to view
@snicoll

This comment has been minimized.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rstoyanchev rstoyanchev rstoyanchev left review comments

Assignees
No one assigned
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Milestone
5.2 M1
Development

Successfully merging this pull request may close these issues.
7 participants
@envious @philsttr @rstoyanchev @eliquinox @benitocm @snicoll @spring-projects-issues